Skip to content

feat(all): make @middy/core optional #511

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Jan 26, 2022
Merged

feat(all): make @middy/core optional #511

merged 5 commits into from
Jan 26, 2022

Conversation

dreamorosi
Copy link
Contributor

@dreamorosi dreamorosi commented Jan 25, 2022 •
edited
Loading

Description of your changes

As detailed in #373 @middy/core has been bundled in all utilities which means customers who don't use the middleware implementations will have an unnecessary dependency bundled in their Lambda deployment package. This PR aims at correcting that.

Changes:

  • Moved @middy/core to devDependency in all utilities that used it (updated also lock file)
  • Changed imports of Middy to include only types
  • Updated documentation to specify that Middy must now be installed with npm i @middy/core
  • Verified that @middy/core is a dependency in examples/cdk

How to verify this change

See checks execution results + e2e result here.

Also ran npm pack on one of the utilities and then installed it in a new dummy project (@middy/core not installed):

~/Codes
❯ mkdir powert-est              

~/Codes
❯ cd powert-est 

~/Codes/powert-est
❯ npm init -y
Wrote to /Users/aamorosi/Codes/powert-est/package.json:

{
  "name": "powert-est",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "keywords": [],
  "author": "",
  "license": "ISC"
}



~/Codes/powert-est
❯ mv ../aws-lambda-powertools-typescript/packages/tracing/aws-lambda-powertools-tracer-0.4.0.tgz .

~/Codes/powert-est
❯ npm i aws-lambda-powertools-tracer-0.4.0.tgz 

added 15 packages, and audited 16 packages in 7s

found 0 vulnerabilities

~/Codes/powert-est 7s
❯ ls -la node_modules 
total 16
drwxr-xr-x  15 aamorosi  staff   480 Jan 25 13:15 .
drwxr-xr-x   6 aamorosi  staff   192 Jan 25 13:15 ..
drwxr-xr-x   3 aamorosi  staff    96 Jan 25 13:15 .bin
-rw-r--r--   1 aamorosi  staff  5329 Jan 25 13:15 .package-lock.json
drwxr-xr-x   4 aamorosi  staff   128 Jan 25 13:15 @aws-lambda-powertools
drwxr-xr-x   4 aamorosi  staff   128 Jan 25 13:15 @aws-sdk
drwxr-xr-x   5 aamorosi  staff   160 Jan 25 13:15 @types
drwxr-xr-x  13 aamorosi  staff   416 Jan 25 13:15 async-hook-jl
drwxr-xr-x  10 aamorosi  staff   320 Jan 25 13:15 atomic-batcher
drwxr-xr-x   8 aamorosi  staff   256 Jan 25 13:15 aws-xray-sdk-core
drwxr-xr-x   9 aamorosi  staff   288 Jan 25 13:15 cls-hooked
drwxr-xr-x   7 aamorosi  staff   224 Jan 25 13:15 emitter-listener
drwxr-xr-x   9 aamorosi  staff   288 Jan 25 13:15 semver
drwxr-xr-x   8 aamorosi  staff   256 Jan 25 13:15 shimmer
drwxr-xr-x  12 aamorosi  staff   384 Jan 25 13:15 stack-chain

Related issues, RFCs

#373

PR status

Is this ready for review?: YES
Is it a breaking change?: YES

Checklist

  • My changes meet the tenets criteria
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • The code coverage hasn't decreased
  • New and existing unit tests pass locally and in Github Actions
  • Any dependent changes have been merged and published in downstream module
  • The PR title follows the conventional commit semantics

Breaking change checklist

  • I have documented the migration process

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@dreamorosi dreamorosi added dependencies Changes that touch dependencies, e.g. Dependabot, etc. utility:all labels Jan 25, 2022
@dreamorosi dreamorosi added this to the production-ready-release milestone Jan 25, 2022
@dreamorosi dreamorosi self-assigned this Jan 25, 2022
@dreamorosi dreamorosi mentioned this pull request Jan 25, 2022
Closed
@dreamorosi
Copy link
Contributor Author

Successful e2e tests execution on this branch.

@dreamorosi dreamorosi linked an issue Jan 25, 2022 that may be closed by this pull request
Maintenance: Make @middy/core a devDependency #373
Closed
2 tasks
saragerion
saragerion reviewed Jan 25, 2022
View reviewed changes
docs/core/logger.md
!!! tip "Using Middy for the first time?"
You can install Middy by running `npm i @middy/core`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the shout out

@dreamorosi dreamorosi merged commit 1107f96 into main Jan 26, 2022
@dreamorosi dreamorosi deleted the feat/all/make_middy_optional branch January 26, 2022 09:55
@dreamorosi dreamorosi mentioned this pull request Jan 10, 2023
Closed
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flochaz flochaz flochaz approved these changes

@saragerion saragerion saragerion approved these changes

Assignees

@dreamorosi dreamorosi

Labels
dependencies Changes that touch dependencies, e.g. Dependabot, etc.
Projects
None yet
Milestone
production-ready-release
Development

Successfully merging this pull request may close these issues.

Maintenance: Make @middy/core a devDependency
3 participants
@dreamorosi @flochaz @saragerion